Skip to content

Conversation

@venusang
Copy link

@venusang venusang commented Aug 28, 2025

Description

This updates the input fields on the User details page so that they are readonly instead of disabled. Additionally it changes the edit button text from Edit form to Edit User details so that it is more descriptive. Lastly, aria-describedby was also added to the button so that it is a11y compliant.

🎟️ FE-112

Screenshots (if appropriate)

fe-112-before-after

How to Test

Navigate to a User in the Users list. Verify that the input fields are readonly (you should be able to highlight the text with your mouse) and that the submit button reflects the text changes shown in the after screenshot.

Checklist

  • I have added before and after screenshots for UI changes
  • I have added JSON response output for API changes
  • I have added steps to reproduce and test for bug fixes in the description
  • I have commented on my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have added a11y-tests label to run a11y audit tests if needed

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
  • If applicable, I've documented the impact of any changes to security controls.
    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@venusang venusang requested a review from a team as a code owner August 28, 2025 15:35
@vercel
Copy link

vercel bot commented Aug 28, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
boundary-ui Ready Ready Preview Comment Dec 2, 2025 5:47pm
boundary-ui-desktop Ready Ready Preview Comment Dec 2, 2025 5:47pm

cameronperera
cameronperera previously approved these changes Aug 28, 2025
Copy link
Collaborator

@cameronperera cameronperera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

ZedLi
ZedLi previously approved these changes Aug 29, 2025
Copy link
Collaborator

@ZedLi ZedLi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the update

hashicc
hashicc previously approved these changes Aug 29, 2025
Copy link
Collaborator

@hashicc hashicc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for helping us with our a11y!

cameronperera
cameronperera previously approved these changes Sep 2, 2025
delete: Delete User
edit-details:
button-text: 'Edit User details'
aria-describedby: 'user-details'
Copy link

@nikkipurcell nikkipurcell Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change aria-describedby to 'edit-user-details' to match button text (if you need aria-describedby).

This adds edit user details translations so the User detail form submit
button is a11y compliant

✅ Closes: FE-112
Per a11y audit, this changes the input fields to readonly instead of
disabled. It also updates the submit button text so that it is context
based and has an aria-label for screenreaders.

✅ Closes: FE-112
This moves the User detail action translation to resources/en-us.yaml
since it is specific to the user resource and is not a generic form
action.

✅ Closes: FE-112
This was not sending the aria-label down to the Edit Users detail button.
However, taking a look further at Hds::Button, this component
does not allow aria-label to be set unless it is an icon only button.
With that said, this cleans up the translation structure and removes the
aria-label translation and mark-up.

✅ Closes: FE-112
Since we cannot apply aria-label to an Hds::Button
when it has text, we can alternatively apply the
aria-describedby attribute instead.  Its value
must match the id of the form.  With that said, a
form id value was also added.

✅ Closes: FE-112
This updates the translation to be more explicit to edit user details
and removes the unnecessary aria at attribute in the form

✅ Closes: FE-112
{{yield to='edit'}}
{{else}}
<Hds::Button
aria-describedby={{@ariaDescribedBy}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we pass in @ariaDescribedBy now?

delete: Delete User
edit-details:
button-text: 'Edit User details'
aria-describedby: 'edit-user-details'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should have a translation for an aria-describedby

{{#if (can 'save model' @model)}}
<form.actions
@enableEditText={{t 'actions.edit-form'}}
@enableEditText={{t 'resources.user.actions.edit-details.button-text'}}
Copy link
Collaborator

@ZedLi ZedLi Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe we need the extra aria-described by stuff since you've already modified the edit text itself to be more descriptive for everyone.

To be clear, I think we have 3 reasonable options here of which only one needs to get done:

  1. Do what you did here and just change the edit text itself to more descriptive
  2. Keep the edit button to just say Edit Form but add an aria-description with the translated string that says Edit Form: Users.
  3. Keep the edit button to just say Edit Form but add an aria-describedby with the ID of the form title, in this case it would be the one here. We would need to add in our own ID for the title so it's not an auto-generated one so we can refer to it with the aria-describedby. We would also need to refactor to remove the DocLink from the title element (and fix the CSS as well) since that content would also be read by the screen reader and instead probably put it in the Badges section like this:
<@header.Title id="header-title">
  {{t 'resources.user.title'}}
</@header.Title>
<@header.Badges>
<DocLink @doc='user' />
</@header.Badges>

Of the three, I think 1 and 2 is easiest but 3 is more convenient as a developer since you don't need to add a new translated string for every edit button, just the ID and refactor of the header. They're all fine at the end of the day though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants